Introduce VortexError::Other and simplify Display implementation#6410
Introduce VortexError::Other and simplify Display implementation#6410
VortexError::Other and simplify Display implementation#6410Conversation
| $crate::VortexError::Context($msg.into(), Box::new($err)) | ||
| ) | ||
| }}; | ||
| (External: $err:expr) => {{ |
There was a problem hiding this comment.
In general - using macros is better than functions here as they don't frames
Merging this PR will not alter performance
Comparing 
Footnotes
|
| InvalidState(..) => "Invalid state error: ", | ||
| Serde(..) => "Serde error: ", | ||
| NotImplemented(..) => "Not implemented error: ", | ||
| MismatchedTypes(..) => "Mismatched types error: ", |
AdamGS
force-pushed
the
adamg/trim-backtraces
branch
from
283ba01 to
87e6d72
Compare
| l => vortex_bail!(InvalidSerde: "invalid decimal byte length: {l}"), | ||
| 2 => DecimalValue::I16(i16::from_le_bytes( | ||
| bytes | ||
| .try_into() |
There was a problem hiding this comment.
This is ugly, I think there are a few variants of VortexError we can get rid of, I'll do that in a followup PR because it might be a bigger change.
|
any idea why |
robert3005
left a comment
robert3005
left a comment
There was a problem hiding this comment.
Let's merge this after Windows test pass, wouldn't want them failing for silly reasons
|
going to try and debug it for a bit, if I can't figure it out and it just some weird windows thing, I'll |
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
AdamGS
force-pushed
the
adamg/trim-backtraces
branch
from
240a374 to
822a5cc
Compare
3 participants
Does this PR closes an open issue or discussion?
This is a long running low burning effort to improve errors.
What changes are included in this PR?
VortexError::Url, VortexError::SerdeJson and VortexError::Invalid state - they are all not used.VortexError::try_from_slice- this is an invariant and we always panic, no reason to treat it as a useful error.Genericerror variant intoExternalandOther, whereExternalis meant to wrap external errors, andOtheris for any error that doesn't match an existing variant.Otheris now also the default ofvortex_errandvortex_bail. They are used widely and often have very different meaning, I think its better to convey vague intent than the wrong one.Displayform to make it clearer what gets printed and how its constructed.What is the rationale for this change?
Error display is a bit more compact when downstream users don't enable backtraces, and I believe this makes maintaining
vortex-errora bit easier.How is this change tested?
In addition to existing tests that verify the result type, I've added a few tests that
Are there any user-facing changes?
One less variant, and
vortex_errandvortex_bailreturn a different type.